-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue-200: Add a new block that can be nested or ability for Query Block to be nested #201
Issue-200: Add a new block that can be nested or ability for Query Block to be nested #201
Conversation
…add-nested-query-block-capability
… feature/issue-200/add-nested-query-block-capability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some suggestions and questions, I'll leave it up to you what you implement vs. not 🍣
@@ -0,0 +1,273 @@ | |||
/* eslint-disable camelcase */ | |||
import { useDebounce } from '@uidotdev/usehooks'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gutenberg has a useDebounce hook and would not require an additional dependency: https://github.com/WordPress/gutenberg/tree/a3d1c078cf88ff84a1d902cd86337dfa83c32642/packages/compose#usedebounce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to keep the existing one, since it operates on the value. The Gutenberg one wants to debounce a function. We can look at refactoring this later if we want.
// It's possible for usePostMetaValue() to run here before useEntityProp() is available. | ||
Boolean(meta?.wp_curate_deduplication), | ||
// @ts-ignore | ||
type ? select('core').getPostType(type) : null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo we should move anything with a @ts-ignore
into the body of the function and ensure that what we're returning is of a stable type. The line above forces the first parameter to be a boolean, but here, it could be a post type object, null, or whatever .getPostType
returns in a failure case (maybe undefined?) - I'd recommend moving the logic that gets the post type object before the return
, validating that it is in fact a post type object, and only including it in the return value if it is actually a post type object, else return null. That way, even though this function is riddled with @ts-ignore
because the Gutenberg project doesn't export types for useSelect
that can be understood here, we can at least guarantee that what comes out of this function is either a post type object or null for the second array member.
components/QueryControls/index.tsx
Outdated
)} | ||
help={__('AND: Posts must have all selected terms. OR: Posts may have one or more selected terms.', 'wp-curate')} | ||
options={andOrOptions} | ||
onChange={(newValue) => setTermRelation(taxonomy.slug, newValue)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally standardize on next
as the new value, and there's a mix of both in this file.
services/deduplicate/index.ts
Outdated
@@ -118,6 +119,22 @@ export function mainDedupe() { | |||
}); | |||
} | |||
|
|||
if (queryBlocks.length !== allQueryBlocks.length) { | |||
allQueryBlocks.forEach((queryBlock) => { | |||
// if (queryBlock.name === 'wp-curate/query' && queryBlock.attributes.query) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
… into feature/issue-200/add-nested-query-block-capability
…add-nested-query-block-capability
Summary
This pull request introduces a new feature addressing the need for a block that can be nested inside a Query block with its own independent settings. This enhancement aims to provide flexibility in content layout, especially for use cases like homepage story arrangements where related stories need to be dynamically adjusted without affecting the main Query block's layout. Fixes #200
Description
The addition of this new block allows for a nested structure within a Query block, enabling separate settings and operations from the outer block. This functionality is particularly useful for displaying a variable number of related stories or backfilled posts in a distinct section that does not influence the layout or settings of the surrounding content. During the development process, efforts were made to ensure that the subquery block could be added and configured with sidebar controls for customization. Debugging was performed to ensure the stability and functionality of these features.
Use Case
A practical application of this feature can be seen in scenarios where a homepage features a top story with several optional related stories. The layout allows for these stories to be independently adjusted in number, without altering the main story's presentation or the arrangement of pinned posts within the outer Query block. This capability ensures greater flexibility and control over content display, catering to various layout needs and editorial decisions. The development process included iterations to refine the user experience in the editor and front-end rendering, ensuring that the subquery block behaves as expected, especially regarding initial rendering and the saving of inner blocks.
For further details, refer to the issue linked below:
Issue #200